Skip to content

fix: split compilation database command strings with shell quoting rules - #10

Open
code-steadfast wants to merge 2 commits into
cuinixam:mainfrom
avengineers:fix/compile-command-quoted-defines
Open

code-steadfast wants to merge 2 commits into
cuinixam:mainfrom
avengineers:fix/compile-command-quoted-defines

Conversation

@code-steadfast

Copy link
Copy Markdown

A compile_commands.json record may carry its flags as command, a single string the shell would have tokenized. Splitting it with str.split() kept the quotes, so an empty define written by CMake as -DMACRO="" reached libclang as the string literal "". A declaration starting with such a macro no longer parsed as a function and disappeared from docs output with no diagnostic.

shlex.split() is not a drop-in replacement: in POSIX mode it treats the backslashes in a Windows compiler path as escapes. Drive shlex directly with escaping disabled instead, so quotes are stripped and backslashes stay literal.

@cuinixam

cuinixam commented Oct 5, 2026

Copy link
Copy Markdown
Owner

I think escape = "" might break string defines on POSIX. If I'm right, CMake writes -DVERSION=\"1.0\", and this would turn it into -DVERSION=\1.0\ instead of -DVERSION="1.0". As far as I know, clang's own compile database reader only disables escapes on Windows. Would it make sense to do the same (if os.name == "nt": lexer.escape = "") and add a POSIX-only case -DVERSION=\"1.0\" expecting -DVERSION="1.0" (with the test cases marked windows-only)?

@code-steadfast
code-steadfast force-pushed the fix/compile-command-quoted-defines branch from 5930646 to cb076ef Compare October 5, 2026 19:37
A compile_commands.json record may carry its flags as `command`, a single
string the shell would have tokenized. Splitting it with `str.split()` kept
the quotes, so an empty define written by CMake as `-DMACRO=""` reached
libclang as the string literal `""`. A declaration starting with such a macro
no longer parsed as a function and disappeared from `docs` output with no
diagnostic.

`shlex.split()` is not a drop-in replacement: in POSIX mode it treats the
backslashes in a Windows compiler path as escapes, so `-IC:\inc` arrives as
`-IC:inc`. Disabling escapes outright is wrong in the other direction, because
a POSIX string define is written `-DVERSION=\"1.0\"` and those backslashes
have to resolve. Drive `shlex` directly and disable escaping only on Windows,
so the backslash rule follows the shell that wrote the command.
@code-steadfast
code-steadfast force-pushed the fix/compile-command-quoted-defines branch from cb076ef to 881a2d6 Compare October 5, 2026 19:54
@code-steadfast

Copy link
Copy Markdown
Author

You are right. Fixed it with if os.name == "nt" and added the POSIX test case for -DVERSION="1.0".

libclang only received the -I and -D options, so headers found through
-isystem were missing. Code inside #if blocks that depend on such a
header was skipped, for example gtest test cases guarded by a KConfig
symbol from autoconf.h.

Also apply ruff-format to one test string.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@cuinixam

cuinixam commented Oct 8, 2026

Copy link
Copy Markdown
Owner

I suggest we do not add the -isystem fix to this pull request. #9 already has the fix and it extends the compile options to support more than just -isystem option.

This branch had an error being deployed

1 failed deployment
release — f2efa653 Deployed Oct 8, 2026 by JuReMq via release #78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants